Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the build arguments through to new_element_path. #43

Merged
merged 1 commit into from Oct 25, 2012

Conversation

seanhandley
Copy link
Contributor

Failing to pass the attributes to new_element_path in the self.build method leads to bad requests for nested element paths.

For example, an api with self.site set to be

'foo.com/customers/:customer_id/items/:item_id'

will generate the url

/customers//items/new.json

when Item.build is called and the API returns an error code as a result.

If we pass the attributes from self.build through to new_element_path, this behaviour is corrected and build uses the correct url to initialise the empty ActiveResource object.

/customers/1/items/new.json

@jeremy
Copy link
Member

jeremy commented Oct 19, 2012

Thanks! Could you add a test case that demonstrates the error & its fix?

@seanhandley
Copy link
Contributor Author

@jeremy I'm happy with this now if you are.

@jeremy
Copy link
Member

jeremy commented Oct 25, 2012

Thanks @seanhandley. Could you push a single squashed commit?

Failing to do this leads to bad requests for nested element paths
so an api with self.site set to be

'foo.com/customers/:customer_id/items/:item_id'

will generate the url

/customers//items/new.json

when Item.build is called and the API returns an error code.

If we pass the attributes from build to new_element_path, this
behaviour is corrected such that build calls the url:

/customers/1/items/new.json
@seanhandley
Copy link
Contributor Author

All done @jeremy

jeremy added a commit that referenced this pull request Oct 25, 2012
Pass the build arguments through to new_element_path.
@jeremy jeremy merged commit d6bbbc3 into rails:master Oct 25, 2012
@mvlwn
Copy link

mvlwn commented Nov 17, 2012

I ran into the same issue. Great to see this was taken care of already. Cheers.

@seanhandley
Copy link
Contributor Author

@mvlwn I'm glad - thanks for saying so!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants